Build JavaCPP libraries at compile time #20
Conversation
| @@ -1,34 +1,40 @@ | |||
| package org.bytedeco.sbt.javacpp | |||
|
|
|||
| import scala.language.postfixOps | |||
| import sbt._ | |||
| import org.bytedeco.javacpp.tools.Builder | |||
There was a problem hiding this comment.
Stupid question: this Builder , which is a plugin-level JavaCPP Builder with a set version based on
Line 26 in cc93495
, isn't necessarily going to support the classes brought in by ""org.bytedeco" % "javacpp" % Versions.javaCppVersion jar", which is a project-level, right?
There was a problem hiding this comment.
Because Versions.javaCppVersion is deducted from the filename from which the Builder class is loaded, it should be the same as what is set in set-javacpp/build.sbt, or project/plugins.sbt of the user project. So the same version of JavaCpp library should be used by the user project and the plugin, unless the user specified a different version in build.sbt instead of project/plugins.sbt. I updated the README documentation on how to specify a different JavaCPP version. It is a breaking change though. A consistency check can be added before JavaCPP build, and print a warning if the library versions do not match.
There was a problem hiding this comment.
@thirstycrow Does that mean we can remove that line with JavaCPP 1.4.3 from sbt-javacpp/built.sbt?
There was a problem hiding this comment.
No, the JavaCPP library is required by both the user project (to define the JavaCPP classes) and the sbt-javacpp plugin (to build the JNI codes).
There was a problem hiding this comment.
Ok, so let's set them to the same version?
There was a problem hiding this comment.
As discussed, we'd need a way to catch and warn the user if the library versions between the build version and dependency versions don't match. Or even perhaps warn if they're incompatible (maybe different minor version? Not sure what build assurances JavaCPP itself provides across versions (cc @saudet), e.g. is JavaCPP 1.5.x reasonably guaranteed to be able to .build() things from all 1.5,x (and fail reliably if it can't)?
README.markdown
Outdated
|
|
||
| ```scala | ||
| javaCppVersion := "1.4.3" | ||
| libraryDependencies += "org.bytedeco" % "javacpp" % "1.5.1" |
There was a problem hiding this comment.
I'd much prefer the mechanism for specifying a different javacpp version to remain the same as it is now: setting a new version, rather than straight up requiring someone to declare the whole dependency. It's both simpler and backwards compatible.
|
I'm not sure I understand, in which circumstances would we need to use 2 different versions of JavaCPP together? I suppose they can get loaded in 2 different class loaders, but then how do they interact? |
thirstycrow
force-pushed
the
build-at-compile-time
branch
2 times, most recently
from
1eb0850 to
c2e05a1
Compare
…s a dependency of the plugin
thirstycrow
force-pushed
the
build-at-compile-time
branch
from
c2e05a1 to
0298ecd
Compare
|
I found a better way to implement the task, using reflection. In this way, the |
|
I see, JavaCPP gets loaded twice, once to get the platform name. Copying
code doesn't sound like a good idea, but we have to do that with Maven
because extensions don't work for transitive dependencies, so I'm guessing
it's the same for sbt?
|
|
I'm not very familiar with how maven works. Can you elaborate why extensions don't work for transitive dependencies with maven? |
|
I think the idea with Maven is that extensions and plugins are only available at build time. When we pull in transitive dependencies, we are not building those, so their build time extensions and plugins don't get used. |
|
Do you mean maven extensions and plugins cannot get a fully resolved dependency tree of the project? |
|
No, at build time, they work fine. At runtime, they are not used, that's the problem. |
|
I see. Sbt has nothing different on this with maven. Ideally, there could be a tiny library for common utilities required both at build time and runtime. For the moment, if you really care about the code duplication, adding javacpp as a dependency of the plugin should still work without conflict, unless the platform determining logic differs between versions used for build and runtime. Should I change it to this style? |
|
Yeah, I don't know what's best. I'll leave it up to @lloydmeta |
| } | ||
|
|
||
| object autoImport { | ||
| type JavaCppBuilder = { |
There was a problem hiding this comment.
If the real org.bytedeco.javacpp.tools.Builder is updated/changed (e.g. when the user configures a newer version), would this still work?
There was a problem hiding this comment.
It should work as long as the corresponding methods that are actually invoked are not missing in the newer version of org.bytedeco.javacpp.tools.Builder.
| import scala.language.{ postfixOps, reflectiveCalls } | ||
| import scala.util.Try | ||
|
|
||
| object JavaCppPlugin extends AutoPlugin { |
There was a problem hiding this comment.
Why did you create a new file?
There was a problem hiding this comment.
This is not required. But when there is another Plugin object, which I'd like to enable/disable, I would not have to specify the package.
| @@ -20,7 +17,28 @@ object Platform { | |||
| */ | |||
| val current: Seq[String] = sys.props.get(platformOverridePropertyKey) match { | |||
| case Some(platform) if platform.trim().nonEmpty => platform.split(' ') | |||
| case _ => Seq(Loader.getPlatform) | |||
| case _ => | |||
There was a problem hiding this comment.
What's the advantage of not pulling in JavaCpp at the sbt level and re-using the loader?
There was a problem hiding this comment.
I chose this style, so I didn't have to think about whether the potential difference of the libraries would cause any trouble. It should work either way, though.
It would be more convenient to build the JavaCPP libs at compile time, so we don't have to setup a C++ build environment at every deploy site.